HybridCache stability and logging improvements #5467

mgravell · 2024-10-03T16:43:19Z

Stability (i.e. when bad things happen, it should be as gentle as possible) and logging:

implement ILogger logging of significant events
add basic EventSource capture
enforce MaximumPayloadBytes, fixes HybridCache MaximumPayloadBytes not operating per documentation #5432
ensure that instability of the underlying cache backend is not exposed to the caller
handle [de]serialization failures appropriately
enforce MaximumKeyLength (do not use cache for invalid keys)

Microsoft Reviewers: Open in CodeFlow

joperezr · 2024-10-03T17:32:00Z

Hey @mgravell, as FYI I have re-targeted this PR to main branch as that is now our 9.0 branch going forward. Nothing should change from your point of view.

- enforce payload quota - enforce key validity - add proper logging (infrastructure failure: needs attn) # Conflicts: # src/Libraries/Microsoft.Extensions.Caching.Hybrid/Microsoft.Extensions.Caching.Hybrid.csproj

- log deserialization failures - expose serialization failures - tests for serialization logging scenarios

...ibraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.MutableCacheItem.cs

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.CacheItem.cs

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.Serialization.cs

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCacheEventSource.cs

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Microsoft.Extensions.Caching.Hybrid.csproj

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCacheEventSource.cs

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/Log.cs

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Microsoft.Extensions.Caching.Hybrid.csproj

test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/LogCollector.cs

test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/HybridCacheEventSourceTests.cs

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/HybridCacheEventSource.cs

BrennanConroy · 2024-10-31T21:30:39Z

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.StampedeStateT.cs

+                            }
+                        }
+                    }
+                    catch (Exception ex)


Do we want to catch the (OperationCanceledException ex) when (SharedToken.IsCancellationRequested) ? (or ex.CancellationToken == SharedToken)

You resolved the conversation without making a change or comment. Would like to know the resolution here.

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.cs

test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/TestEventListener.cs

...crosoft.Extensions.Telemetry.Abstractions/Microsoft.Extensions.Telemetry.Abstractions.csproj

mgravell · 2024-10-31T22:32:44Z

Pretty sure the CI failure is timing; will tweak tomorrow

* - handle serialization failures - enforce payload quota - enforce key validity - add proper logging (infrastructure failure: needs attn) # Conflicts: # src/Libraries/Microsoft.Extensions.Caching.Hybrid/Microsoft.Extensions.Caching.Hybrid.csproj * - add "callback" to .dic - log deserialization failures - expose serialization failures - tests for serialization logging scenarios * support and tests for stability despite unreliable L2 * nit * Compile for NS2.0 * include enabled check in our log output * add event-source tracing and counters * explicitly specify event-source guid * satisfy the stylebot overloads * nix SDT * fix failing CI test * limit to net462 * PR feedback (all except event tests) * naming * add event source tests * fix redundant comment * add clarification * more clarifications * dance for our robot overlords * drop Microsoft.Extensions.Telemetry.Abstractions package-ref * fix glitchy L2 test * better tracking for invalid event-source state * reserve non-printable characters from keys, to prevent L2 abuse * improve test output for ETW * tyop * ETW tests: allow longer if needed * whitespace * more ETW fixins --------- Co-authored-by: Jose Perez Rodriguez <[email protected]>

dotnet-policy-service bot assigned mgravell Oct 3, 2024

joperezr changed the base branch from dev to main October 3, 2024 17:31

RussKie requested a review from a team October 9, 2024 04:55

RussKie added the area-caching-hybrid label Oct 10, 2024

mgravell and others added 6 commits October 22, 2024 11:03

- handle serialization failures

a874d1c

- enforce payload quota - enforce key validity - add proper logging (infrastructure failure: needs attn) # Conflicts: # src/Libraries/Microsoft.Extensions.Caching.Hybrid/Microsoft.Extensions.Caching.Hybrid.csproj

- add "callback" to .dic

3951c72

- log deserialization failures - expose serialization failures - tests for serialization logging scenarios

support and tests for stability despite unreliable L2

aac82e8

nit

6fd3047

Compile for NS2.0

8a7b59b

include enabled check in our log output

5605ce5

mgravell force-pushed the marc/max_size branch from bab8ba8 to 5605ce5 Compare October 22, 2024 10:04

add event-source tracing and counters

d562906

mgravell marked this pull request as ready for review October 22, 2024 12:33

mgravell requested a review from a team as a code owner October 22, 2024 12:33

mgravell added 5 commits October 22, 2024 13:46

explicitly specify event-source guid

6a78eb2

satisfy the stylebot overloads

3c9c88d

nix SDT

cbe7eb7

fix failing CI test

ab98bab

limit to net462

607d395

amcasey reviewed Oct 25, 2024

View reviewed changes

BrennanConroy reviewed Oct 25, 2024

View reviewed changes

mgravell added 4 commits October 28, 2024 10:11

PR feedback (all except event tests)

8b59367

naming

43e0406

add event source tests

cb370ee

fix redundant comment

028d813

mgravell requested review from BrennanConroy and amcasey October 29, 2024 11:47

mgravell added 2 commits October 29, 2024 11:49

add clarification

91fca21

more clarifications

1a4520b

mgravell added 8 commits October 29, 2024 12:06

dance for our robot overlords

6b7bb54

drop Microsoft.Extensions.Telemetry.Abstractions package-ref

faba196

fix glitchy L2 test

8d757c7

Merge branch 'main' into marc/max_size

923ad11

better tracking for invalid event-source state

6301b08

reserve non-printable characters from keys, to prevent L2 abuse

e98d8d7

improve test output for ETW

d1a90fa

tyop

8989fd9

BrennanConroy approved these changes Oct 31, 2024

View reviewed changes

mgravell added 4 commits November 1, 2024 16:28

ETW tests: allow longer if needed

03001c2

whitespace

031d9eb

more ETW fixins

3488a87

Merge branch 'main' into marc/max_size

5a4d146

mgravell merged commit 9eea77d into dotnet:main Nov 4, 2024
6 checks passed

mgravell mentioned this pull request Nov 5, 2024

HybridCache: don't log cancellation as failure event #5601

Merged

joperezr mentioned this pull request Nov 5, 2024

[release/9.0] Merging changes from main into release branch for November release #5602

Merged

github-actions bot locked and limited conversation to collaborators Dec 5, 2024

HybridCache stability and logging improvements #5467

HybridCache stability and logging improvements #5467

Uh oh!

Conversation

mgravell commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

joperezr commented Oct 3, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BrennanConroy Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

BrennanConroy Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mgravell commented Oct 31, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mgravell commented Oct 3, 2024 •

edited

Loading